-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove Tab.TabContent.url #2647
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DuckDuckGo/Fireproofing/Extensions/FireproofingURLExtensions.swift
Outdated
Show resolved
Hide resolved
|
||
/// user-editable URL displayed in the address bar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename it to urlForAddressBar
? 🤷 Not a strong opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don‘t know, I think user editable url
is pretty expressive to keep it
let urlScheme = tabViewModel.tab.content.url?.scheme | ||
let isHypertextUrl = urlScheme == "http" || urlScheme == "https" | ||
let isHypertextUrl = if case .url(let url, _, _) = tabViewModel.tab.content, | ||
!(url.isDuckPlayer || url.isDuckURLScheme), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if a computed property for this condition would make sense for reuse in future since these URLs will always have a special treatment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refined it a little bit for more clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better 👍 thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -30,6 +30,11 @@ extension URL.NavigationalScheme { | |||
return [.http, .https, .file] | |||
} | |||
|
|||
/// HTTP or HTTPS | |||
var isHypertextScheme: Bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
let urlScheme = tabViewModel.tab.content.url?.scheme | ||
let isHypertextUrl = urlScheme == "http" || urlScheme == "https" | ||
let isHypertextUrl = if case .url(let url, _, _) = tabViewModel.tab.content, | ||
!(url.isDuckPlayer || url.isDuckURLScheme), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better 👍 thank you!
Actually it seems we're allowing to bookmark any url, see the design review |
I‘ve fixed opening |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! ✅
…utdown # By Dax the Duck (9) and others # Via GitHub (6) and others * main: (47 commits) Update BSK to 141.1.1 (#2713) macOS: VPN Metadata Improvements (#2704) duck page suggestions (#2666) macOS: Add pixels to track VPN wake and stop attempts (#2694) Trusted url indicator (#2665) remove Tab.TabContent.url (#2647) Bump version to 1.85.0 (176) Update release automation to support Privacy Pro section in release notes (#2710) Fix for VPN stop issues. (#2689) Hard-codes the VPN waitlist flags to ON (#2709) Add subscription status to the macOS metadata (#2680) Bump version to 1.85.0 (175) Call finish in the correct place (#2702) Fix layout issue on DBP (#2700) Update autoconsent to v10.6.1 (#2618) Make Clear All History shortcut available without entering Main Menu (#2682) Fix DataBrokerProtectionProcessor.swift lint Fix SwiftLint Add parameter allowed encoding to error descriptions (#2691) Remove debug flags ... # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Task/Issue URL: https://app.asana.com/0/0/1207078384259685/f
Description:
urlForWebView
anduserEditableUrl
Steps to test this PR:
Internal references:
Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation